-
-
Notifications
You must be signed in to change notification settings - Fork 224
reiterate version inference #1202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
reiterate version inference #1202
Conversation
…ic version verification - Introduced should_infer method in PyProjectData to determine if version inference should proceed based on configuration. - Updated version_inference.py to utilize should_infer for handling version inference conditions. - Modified tests to reflect changes in dynamic version verification and ensure proper error handling when dynamic=['version'] is missing.
- enable dependency injecting supposed pyporject data - migrate tests to ue the api directly instead of writing
…bility - Updated the path representation in the pytest report header to replace 'site-packages' with 'site:.' and the current working directory with 'CWD:.' for improved clarity.
…missing sections - Introduced a class method `empty` in PyProjectData for creating empty instances. - Simplified the `read_pyproject` function to utilize the new `empty` method when the configuration file is missing. - Enhanced logging for missing tool sections in the TOML configuration.
- Introduced `InvalidTomlError` to handle parsing errors in TOML files. - Simplified the `read_pyproject` function by removing unnecessary exception handling for file not found. - Updated exception handling in version inference to use `InvalidTomlError` for better clarity in error logging.
- Introduced `_given_result` parameter in `read_pyproject` - expand the setuptools integration points with it - Improved error handling by allowing direct injection of `PyProjectData`, `InvalidTomlError`, or `FileNotFoundError` for better testability.
- drop magic boolean parameters - drop unused exception creator - simplify calls
the decission tree is massively simplified
…t and the part that doesnt
Its used when using the simplified activation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reworks the version inference logic for setuptools-scm, moving towards better unit testing of inference conditions and introducing a new "simple" extra for streamlined activation. The main changes simplify and consolidate version inference decision-making while removing overly aggressive auto-activation that caused issues with legacy projects.
Key changes:
- Introduces simplified activation via
setuptools-scm[simple]
extra with explicit opt-in - Consolidates version inference logic in
get_version_inference_config
with cleaner decision paths - Replaces complex E2E integration tests with focused unit tests using
PyProjectData.for_testing()
Reviewed Changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
testing/test_version_inference.py | Complete rewrite with parametrized unit tests and helper functions for cleaner inference testing |
testing/test_pyproject_reading.py | Adds unit tests for the new has_build_package_with_extra function and removes missing file handling |
testing/test_integration.py | Removes many E2E tests in favor of unit-level testing, keeps essential integration scenarios |
src/setuptools_scm/_integration/version_inference.py | Simplifies decision logic and adds infer_version_string pure helper function |
src/setuptools_scm/_integration/pyproject_reading.py | Adds should_infer() method and support for simple extra detection |
pyproject.toml | Adds the new simple extra to project configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| VersionInferenceWarning | ||
| VersionInferenceNoOp, | ||
) -> None: | ||
"""Helper to test get_version_inference_config and assert expected result type.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of 'tracebackhide = True' should be documented. Add a comment explaining that this hides the helper function from pytest tracebacks to improve test failure readability.
"""Helper to test get_version_inference_config and assert expected result type.""" | |
"""Helper to test get_version_inference_config and assert expected result type.""" | |
# Hide this helper function from pytest tracebacks to improve test failure readability. |
Copilot uses AI. Check for mistakes.
VersionInferenceNoOp, # Don't infer (silent) | ||
] | ||
|
||
|
||
def infer_version_string( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function docstring should clarify the relationship between this function and the existing integration points. Consider adding an example usage to make the pure helper nature more apparent.
Copilot uses AI. Check for mistakes.
aftermath of #1150
turns out legacy projects have many variables to enable inference easy
now we depend on the section again
im also moving towards better unit-testing of the conditions as i eventually want to bring it back as a extra
closes #1198
closes #1201
closes #1150 by adding the simple extra for a opt in without section